Skip to content

Client-side of static invoice server #3618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

valentinewallace
Copy link
Contributor

As part of being an async recipient, we need to interactively build an offer and static invoice with an always-online node that will serve static invoices on our behalf in response to invoice requests from payers.

While users could build this invoice manually, the plan is for LDK to automatically build it for them using onion messages. See this doc for more details on the protocol. Here we implement the client side of the linked protocol.

See lightning/bolts#1149 for more information on async payments.

@valentinewallace
Copy link
Contributor Author

Will go through the commits in a bit more detail before taking this out of draft, but conceptual feedback or feedback on the protocol itself is welcome, or the way the code is organized overall. It does add a significant amount of code to ChannelManager currently.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I wonder if we shouldn't allow the client to cache N offers rather than only 1. I worry a bit about the privacy implications of having One Offer that gets reused across different contexts.

///
/// [`StaticInvoice`]: crate::offers::static_invoice::StaticInvoice
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
offer: Offer,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand why this has to be here explicitly? Doesn't the StaticInvoice contain the Offer('s fields) in it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As implemented, the async recipient will never cache the StaticInvoice themselves. The recipient creates the invoice, sends it to the server, and includes the Offer in the reply path to cache once the invoice is confirmed as persisted. I think the google doc is out of date here, sorry.

This made sense to me while implementing because we don't want to cache the offer until it's confirmed as persisted by the server and I couldn't think of a reason why the client would need to have to the static invoice.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, I misread which context this was (don't we need a context for sending the static invoice to the server? I guess that's a separate PR), this makes sense.

@valentinewallace
Copy link
Contributor Author

Hmmm, I wonder if we shouldn't allow the client to cache N offers rather than only 1. I worry a bit about the privacy implications of having One Offer that gets reused across different contexts.

I think that makes sense, so they would interactively build and cache a few and then randomly(?) return one of them on get_cached_async_receive_offer?

It seems reasonable to save for follow-up although I could adapt the AsyncReceiveOffer cache struct serialization for this now.

Comment on lines 12709 to 12714
// Expire the offer at the same time as the static invoice so we automatically refresh both
// at the same time.
let offer_and_invoice_absolute_expiry = Duration::from_secs(core::cmp::min(
offer_paths_absolute_expiry.as_secs(),
duration_since_epoch.saturating_add(STATIC_INVOICE_DEFAULT_RELATIVE_EXPIRY).as_secs()
));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I want to address eventually (but maybe not in this PR) is that right now we cap the expiry of our offer/static invoice at 2 weeks, which doesn't work well for the "offer in Twitter bio" use case. Probably we can add something to UserConfig for this, and expose a method for users to proactively rotate their offer if it never expires?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if for that we shouldn't try to come up with a scheme to allow the offer to last longer than the static invoice? I mean ideally an offer lasts at least a few years, but it kinda could cause you just care about the storage server being reliable, you don't care much about the static invoice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. We could include another set of long-lived paths in the OfferPaths message that allows the recipient to refresh their invoice later while keeping the same offer [paths].

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean maybe the OffersPath paths should just be super long-lived? I don't see a strong reason to have some concept of long-lived paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the OfferPaths offer_paths are super long lived, we still need a way for the recipient to update their static invoice later. So the additional paths would be for that purpose, is my thinking.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh vs just having the original paths be long-lived? I guess we could, but it seems like we could just make all the paths long-lived?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. In the current PR, the recipient sends PersistStaticInvoice over the reply path to the OfferPaths message, and that reply path is short-lived.

So we could make that reply path long-lived instead and have the recipient cache that reply path to update their invoice later. Just to confirm, that's what you're suggesting?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that's what I was thinking. Basically just make it a "multi-reply path"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. It means we won't get extra message attempts over alternate blinded paths, but that might be a premature optimization anyway, hard to tell.

@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented Feb 25, 2025

I think that makes sense, so they would interactively build and cache a few and then randomly(?) return one of them on get_cached_async_receive_offer?

Yea, I dunno what to do for the fetch'er, maybe we just expose the whole list?

It seems reasonable to save for follow-up although I could adapt the AsyncReceiveOffer cache struct serialization for this now.

Makes sense, tho I imagine it would be a rather trivial diff, no?

@jkczyz jkczyz self-requested a review February 27, 2025 18:10
@valentinewallace valentinewallace added the weekly goal Someone wants to land this this week label Feb 27, 2025
@@ -1057,6 +1057,7 @@ impl_for_vec!(crate::chain::channelmonitor::ChannelMonitorUpdate);
impl_for_vec!(crate::ln::channelmanager::MonitorUpdateCompletionAction);
impl_for_vec!(crate::ln::channelmanager::PaymentClaimDetails);
impl_for_vec!(crate::ln::msgs::SocketAddress);
impl_for_vec!(crate::blinded_path::message::BlindedMessagePath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might not belong in 9e0f2da?

Comment on lines 154 to 157
/// Once [`Self::invoice`] has been persisted, these paths should be used to send
/// [`StaticInvoicePersisted`] messages to the recipient to confirm that the offer corresponding
/// to the invoice is ready to receive async payments.
pub invoice_persisted_paths: Vec<BlindedMessagePath>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using the reply path be sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be, based on discussion here #3618 (comment), yeah.

I was hesitant to use a Responder param in the handler for this message because it's not intended to be used immediately or returned with .respond_with(..) like in its other uses. The intention is for servers handling this message to go persist the invoice in their db and send a separate OM later once that's done.

With that context, thoughts on this API?

	fn handle_serve_static_invoice(
		&self, message: ServeStaticInvoice, context: AsyncPaymentsContext,
		reply_path: BlindedMessagePath
	);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be, based on discussion here #3618 (comment), yeah.

I was hesitant to use a Responder param in the handler for this message because it's not intended to be used immediately or returned with .respond_with(..) like in its other uses. The intention is for servers handling this message to go persist the invoice in their db and send a separate OM later once that's done.

Right, but that discussion is about long-lived paths, IIUC. But here it's simply an async response because of the database write.

With that context, thoughts on this API?

	fn handle_serve_static_invoice(
		&self, message: ServeStaticInvoice, context: AsyncPaymentsContext,
		reply_path: BlindedMessagePath
	);

I think using the responder is fine. It's serializable so can be used later with ResponseInstruction::into_instructions to get MessageSendInstructions, which then can be passed to OnionMessenger::send_onion_message to send asynchronously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but that discussion is about long-lived paths, IIUC. But here it's simply an async response because of the database write.

Right sorry, I just meant that prior to this PR reply paths were used in a fairly narrow context and we're expanding that here.

I think using the responder is fine. It's serializable so can be used later with ResponseInstruction::into_instructions to get MessageSendInstructions, which then can be passed to OnionMessenger::send_onion_message to send asynchronously.

Sounds good!

Comment on lines +568 to +577
const IV_BYTES: &[u8; IV_LEN] = b"LDK Offer Paths~";
let mut hmac = expanded_key.hmac_for_offer();
hmac.input(IV_BYTES);
hmac.input(&nonce.0);
hmac.input(ASYNC_PAYMENTS_OFFER_PATHS_INPUT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to include path_absolute_expiry?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the nonce/IV was sufficient but I'm not certain. @TheBlueMatt would it be an improvement to commit to the expiry in the hmac? IIUC the path still can't be re-purposed...

@valentinewallace valentinewallace marked this pull request as ready for review March 4, 2025 21:14
@valentinewallace
Copy link
Contributor Author

Going to base this on #3640. Will finish updating the ser macros based on those changes and push updates here after finishing some review.

@valentinewallace valentinewallace removed the weekly goal Someone wants to land this this week label Mar 4, 2025
@@ -4827,6 +4880,53 @@ where
)
}

#[cfg(async_payments)]
fn check_refresh_async_receive_offer(&self) {
Copy link
Contributor

@joostjager joostjager Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My view on channelmanager is that it manages the channels. Relatively low level operations. I think that async offers live on a higher level, a level above basic channel management. Shouldn't this be reflected in code, by implementing it outside of channel manager?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I believe this will move into OffersMessageFlow once #3639 lands!

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple of comments that I find out while working on the CI failure in #3593

shaavan and others added 11 commits April 18, 2025 18:29
Encapsulates logic for fetching peers used in blinded path creation.
Reduces duplication and improves reusability across functions.
These functions will be used in the following commit to replace closure usage
in Flow trait functions.
As part of being an async recipient, we need to support interactively building
an offer and static invoice with an always-online node that will serve static
invoices on our behalf.

Add a config field containing blinded message paths that async recipients can
use to request blinded paths that will be included in their offer. Payers will
forward invoice requests over the paths returned by the server, and receive a
static invoice in response if the recipient is offline.
Because async recipients are not online to respond to invoice requests,
the plan is for another node on the network that is always-online to serve
static invoices on their behalf.

The protocol is as follows:
- Recipient is configured with blinded message paths to reach the static invoice
  server
- On startup, recipient requests blinded message paths for inclusion in their
  offer from the static invoice server over the configured paths
- Server replies with offer paths for the recipient
- Recipient builds their offer using these paths and the corresponding static
  invoice and replies with the invoice
- Server persists the invoice and confirms that they've persisted it, causing
  the recipient to cache the interactively built offer for use

At pay-time, the payer sends an invoice request to the static invoice server,
who replies with the static invoice after forwarding the invreq to the
recipient (to give them a chance to provide a fresh invoice in case they're
online).

Here we add the requisite trait methods and onion messages to support this
protocol.
In future commits, as part of being an async recipient, we will interactively
build offers and static invoices with an always-online node that will serve
static invoices on our behalf.

Once an offer is built and the static invoice is confirmed as persisted by the
server, we will use the new offer cache added here to save the invoice metadata
and the offer in ChannelManager, though the OffersMessageFlow is responsible
for keeping the cache updated.
As an async recipient, we need to interactively build static invoices that an
always-online node will serve to payers on our behalf.

At the start of this process, we send a requests for paths to include in our
offers to the always-online node on startup and refresh the cached offers when
they expire.
As an async recipient, we need to interactively build a static invoice that an
always-online node will serve to payers on our behalf.

As part of this process, the static invoice server sends us blinded message
paths to include in our offer so they'll receive invoice requests from senders
trying to pay us while we're offline. On receipt of these paths, create an
offer and static invoice and send the invoice back to the server so they can
provide the invoice to payers.
As an async recipient, we need to interactively build a static invoice that an
always-online node will serve on our behalf.

Once this invoice is built and persisted by the static invoice server, they
will send us a confirmation onion message. At this time, cache the
corresponding offer and mark it as ready to receive async payments.
As an async recipient, we need to interactively build offers and corresponding
static invoices, the latter of which an always-online node will serve to payers
on our behalf.

Offers may be very long-lived and have a longer expiration than their
corresponding static invoice. Therefore, persist a fresh invoice with the
static invoice server when the current invoice gets close to expiration.
Over the past several commits we've implemented interactively building an async
receive offer with a static invoice server that will service invoice requests
on our behalf as an async recipient.

Here we add an API to retrieve the resulting offers so we can receive payments
when we're offline.
@valentinewallace valentinewallace force-pushed the 2025-02-static-inv-server-client branch from 5cf3585 to 5455d55 Compare May 14, 2025 19:30
@valentinewallace
Copy link
Contributor Author

Pushed some updates after moving the async receive offer cache into the new OffersMessageFlow struct added in #3639.

pub(super) fn should_build_offer_with_paths(
&mut self, message: &OfferPaths, duration_since_epoch: Duration,
) -> bool {
let needs_new_offers = self.check_expire_offers(duration_since_epoch);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: probably don't mutate state in a getter method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants